Skip to content

fix(runtime-host): forward the durable steering echo to session subscribers - #3316

Open
me2seeks wants to merge 5 commits into
apache:mainfrom
me2seeks:fix/3304-steering-transcript-projection
Open

fix(runtime-host): forward the durable steering echo to session subscribers#3316
me2seeks wants to merge 5 commits into
apache:mainfrom
me2seeks:fix/3304-steering-transcript-projection

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #3304 — a steering message sent while the TUI is mid-turn was delivered to the model but never rendered in the transcript.

Root cause: the Runtime Host event pump dropped the durable steering_message (it is not a transient tool/text event), leaving observation of the transient in_flight queue state as the only live render path. The async, coalesced canonical refresh can skip that state entirely (queued → consumed), so subscribers saw only an emptying queue_update and the interjection vanished from the UI.

Changes

  • Protocol (session-continuity.ts): new SessionSteeringEvent carried on the existing subscription.session_event frame (no toolUseId), strict exact-keys decode with bounded message content; wire schema 4 → 5 (mismatched clients already fail fast at subscription open).
  • Host (root-turn-coordinator.ts, session-continuity-coordinator.ts): the pump now forwards the durable steering_message; projectToolEventprojectSessionEvent passes it through verbatim to subscribers.
  • Client projector (adapter/session-projector.ts): projects the forwarded echo as a first-class steering_message SessionEvent, and dedups per messageId against the existing queue in-flight synthesis (#renderedSteeringMessageIds) — whichever authoritative path arrives first renders exactly once. The synthesis stays for the attach/rejoin window where the durable event has not landed yet.

TUI reducer and desktop live projection already render steering_message, so no surface changes were needed there.

Tests

  • New: projector regression (in-flight state never observed → echo still renders; bidirectional exactly-once dedup; rejoin seed), protocol round-trip + rejection cases, coordinator forwarding, and a full-stack test (real host + fake backend + subscribed client receives the steering frame).
  • packages/runtime-host: 1024/1024 pass; packages/cli steering/transcript/driver suites: 202/202; @maka/ui: 183/183; biome check clean; desktop typecheck clean.

Compatibility

Wire schema bump 4 → 5: old clients connecting to a new host fail the handshake at subscription open instead of mis-decoding the new event variant (the established closed wire-schema convention).

AI use

  • Generative tooling made a substantive contribution
  • No generative tool made a substantive contribution

Tool(s) and scope: Maka (AI coding agent) authored the implementation and tests; the diff was human-reviewed before push. Generated-by: Maka trailers are present on the branch commits.

Astro-Han
Astro-Han previously approved these changes Aug 20, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE.

The fix is at the right seam. The root cause is stated precisely and the change addresses it rather than the symptom: the pump now forwards the durable steering_message, so rendering no longer depends on a subscriber happening to observe a transient in_flight queue state that the coalesced canonical refresh is free to skip.

I checked the two things that could have made this wrong, and neither does:

  • The transient classification does not affect persistence. Adding steering_message to isRuntimeSessionTransientEvent looked like it might drop the message from durable storage. It cannot: :2099 is the predicate's only call site, and it gates continuity.acceptRuntimeEvent — live forwarding — while persistence happens upstream in the run's own event stream. Only the name is now misleading, which is the P3 inline.
  • The mixed-version story holds without an epoch bump. SESSION_CONTINUITY_SCHEMA_VERSION goes 4 to 5, and the check lives inside decodeSessionContinuitySnapshot (session-continuity.ts:521), which the subscription.open result decode runs — so a schema-4 client fails at open and never reaches a frame carrying the new variant. The precedent is real and recent: #3074 took 3 to 4 the same way without touching the epoch. Worth stating explicitly since #3236, #3199, #3133 and #2521 are currently all contending for epoch 28 — this PR correctly stays out of that.

The dedup is the part I looked at hardest and it holds up. #renderedSteeringMessageIds is set on all three render paths — the rejoin seed at :154, the newly-in-flight synthesis at :356, and the forwarded echo at :329 — and the tests pin the race in both orders, plus the rejoin case, plus the original bug's shape (in-flight state never observed). The protocol test covers a round trip, the toolUseId rejection and the oversize-content rejection. The full-stack test in execution-host-message.test.ts asserts against a real host with a subscribed client rather than an intermediate object.

biome format is clean on all eight files at this head.

One P3 inline, purely naming. Nothing blocking.

AI disclosure: this review was assisted by Claude (Opus) for code search and cross-checking. Everything above I re-derived myself against the source at 18e2905f9.

Comment thread packages/runtime-host/src/server/root-turn-coordinator.ts
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 20, 2026
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.
@Astro-Han

Copy link
Copy Markdown
Contributor

The test failure here is not caused by anything in this PR. The build step fails at src/__tests__/execution-host-message.test.ts(101,54): error TS2554: Expected 1 arguments, but got 2.

#3277 (a6f33c952, "retire client surface identity plumbing") landed after this head and removed connectClient's second surface parameter, while the new test added here still calls connectClient(fixture.root, 'tui'). A rebase onto main and dropping the 'tui' argument should be the whole fix.

My approval stands — the diff I reviewed is unaffected.

Written with Claude Code; the CI log, #3277's diff and the current connectClient signature were read at this head and verified by me.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the delta from 18e2905f9 — one commit, three files, a pure rename. It is the right response to the naming concern and I have no objection to it.

RuntimeSessionTransientEventRuntimeSessionForwardedEvent and isRuntimeSessionTransientEventisRuntimeSessionForwardedEvent make the predicate say what it decides instead of implying a durability class it never controlled, and the new comment states the actual rule — forward live to subscribers via the continuity coordinator rather than letting the canonical refresh carry it, with persistence orthogonal and handled upstream in the run's own event stream. That is exactly the distinction that made steering_message's membership look wrong under the old name, and it is now settled by the name rather than by having to re-derive it. projectToolEventprojectSessionEvent in the projector is the same correction one layer down; the function has never been tool-specific.

I checked the rename is complete rather than partial: zero occurrences of either old identifier remain in the three touched files at this head, and the two module-local projectSessionEvent functions live in different modules at different layers, so there is no shadowing.

The one blocker is unchanged and is now inside this PR's scope — see the thread below. My earlier approval was dismissed by the new commit, so this is a fresh review; the P1 is the only thing keeping it a COMMENT.

Worth flagging separately: CI has not run on this head at all. The run is sitting at action_required (id 32367571694), which is the fork-contributor workflow-approval gate, not a failure. A committer has to release it before any check result exists — so the rebase alone will not produce a green check without that.

AI disclosure: reviewed with Claude Code. I read the delta and the current connectClient signature at this head myself, confirmed the rename left no stale identifiers, and checked the workflow-run state directly. The conclusions are mine.

Comment thread packages/runtime-host/src/__tests__/execution-host-message.test.ts Outdated
Comment thread packages/runtime-host/src/adapter/session-projector.ts
@me2seeks
me2seeks force-pushed the fix/3304-steering-transcript-projection branch from 2225aea to 6397a56 Compare August 20, 2026 14:09
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 20, 2026
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 20, 2026
…ady rendered it

subscription.open can bootstrap the durable steering message and install the
subscriber before the Host's forwarded echo arrives; seed the render-dedup
set from steering messages already durable in the transcript so the bootstrapped
render stays the only one. Also rebased onto main and updated the new e2e test
to the post-apache#3277 single-argument connectClient.

Addresses review on apache#3316
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 22, 2026
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.

Generated-by: Maka
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 22, 2026
…ady rendered it

subscription.open can bootstrap the durable steering message and install the
subscriber before the Host's forwarded echo arrives; seed the render-dedup
set from steering messages already durable in the transcript so the bootstrapped
render stays the only one. Also rebased onto main and updated the new e2e test
to the post-apache#3277 single-argument connectClient.

Addresses review on apache#3316

Generated-by: Maka
@me2seeks
me2seeks force-pushed the fix/3304-steering-transcript-projection branch from 6397a56 to 3ebc304 Compare August 22, 2026 13:43
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 22, 2026
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.

Generated-by: Maka
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 22, 2026
…ady rendered it

subscription.open can bootstrap the durable steering message and install the
subscriber before the Host's forwarded echo arrives; seed the render-dedup
set from steering messages already durable in the transcript so the bootstrapped
render stays the only one. Also rebased onto main and updated the new e2e test
to the post-apache#3277 single-argument connectClient.

Addresses review on apache#3316

Generated-by: Maka
@me2seeks
me2seeks force-pushed the fix/3304-steering-transcript-projection branch from 3ebc304 to be9d908 Compare August 22, 2026 18:35

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnosis here is the good kind — the durable ledger always had the steering message, so nothing was lost at rest; what was missing was the subscriber hop. Forwarding the durable event over the existing subscription.session_event channel rather than inventing a new one is the right shape, and de-duplicating by messageId across bootstrap, in-flight synthesis and live echo is what makes the three paths safe to keep. Keeping the synthesis path is also correct — it still covers the attach/rejoin window where no echo arrives.

One blocker, and it's the housekeeping that a protocol change always drags along. Inline.

I checked the two earlier P1s against this head and both are closed: connectClient is single-argument now, and the bootstrap de-duplication landed with test coverage. I didn't re-raise them.

On the three things I'd normally worry about for a forwarding change — delivery failure, reconnect, and double delivery — the state passes through queue lease → kernel persist → RuntimeEvent with refs.providerEventId = messageId → pump → subscription frame → projector, and the messageId identity survives end to end, which is what makes the de-duplication sound. With no subscriber the event simply isn't enqueued and stays in the run stream. acceptRuntimeEvent does throw when the canonical root turn doesn't match, which interrupts the pump — but that's the same door text_delta already goes through, so it isn't a seam this PR opens.

} from './session-transcript.js';

export const SESSION_CONTINUITY_SCHEMA_VERSION = 4 as const;
export const SESSION_CONTINUITY_SCHEMA_VERSION = 5 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] test is red on this head, and the gate is right.

Protocol files changed but RUNTIME_HOST_COMPATIBILITY_EPOCH is still 39

This PR takes SESSION_CONTINUITY_SCHEMA_VERSION from 4 to 5 and makes decodeSessionContinuitySnapshot reject anything that isn't 5, so an older client fails at open. That's a deliberate compatibility break and the right call — but RUNTIME_HOST_COMPATIBILITY_EPOCH in protocol/index.ts:94 is still 39, the same value as main.

Worth noting this isn't an assertion mismatch, so the usual "+ is actual, - is expected" reading doesn't apply — the gate is asking for an epoch the current base hasn't seen, and it hasn't got one.

Bumping the epoch alongside the schema is all that's needed. It's the piece that lets the host tell an incompatible client apart from a merely older one, which is exactly the situation this schema bump creates.

me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 23, 2026
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.

Generated-by: Maka
me2seeks added a commit to me2seeks/maka-agent that referenced this pull request Aug 23, 2026
…ady rendered it

subscription.open can bootstrap the durable steering message and install the
subscriber before the Host's forwarded echo arrives; seed the render-dedup
set from steering messages already durable in the transcript so the bootstrapped
render stays the only one. Also rebased onto main and updated the new e2e test
to the post-apache#3277 single-argument connectClient.

Addresses review on apache#3316

Generated-by: Maka
@me2seeks
me2seeks force-pushed the fix/3304-steering-transcript-projection branch from be9d908 to 323fb32 Compare August 23, 2026 10:01
@me2seeks

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (6ada6b546+, 3 commits replayed cleanly). The CI failure you flagged is resolved by the rebase itself: the branch's execution-host-message test no longer passes a surface argument to connectClient, and the full clean rebuild + runtime-host suite is green on the new head (1090/1090, biome clean).

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head 323fb3220eb6722853f9ac185ec491deef75911f. NO-GO at this head: one [P1] inline (the epoch gate), and test is terminal failure on this exact head, so the approve condition fails on both findings and checks.

This PR has two prior review rounds (2026-08-20 approved at 18e2905f, later dismissed on head movement; 2026-08-22 commented at be9d908f). The 08-22 round's open item was exactly the epoch bump; the head has since been rebased (now on base 6ada6b54) but the epoch is still untouched, and the red check is the proof.

This pass re-verified the substance independently, on this head:

  • The identity chain behind the de-duplication holds. Transcript user message id comes from stableMessageId = refs.storedMessageId ?? refs.providerEventId (runtime-event-read-model.ts), the durable echo carries messageId = lease.messageId, and the queue in-flight entries are keyed by the same messageId - so the three-way dedup (#renderedSteeringMessageIds) compares like with like across bootstrap, synthesis, and echo.
  • The new projector tests cover the interesting races: echo when the coalesced refresh skipped in_flight, exactly-once in both directions, rejoin seed, and bootstrap suppression of a late echo. I read them against the production id sources and they match.
  • The wire change is bounded: SessionSteeringEvent decodes with exact keys, ids/counts validated, and content through decodeMessageContent (text + whole-content byte caps). SESSION_CONTINUITY_SCHEMA_VERSION 4 -> 5 with reject-at-open is the established convention.
  • The rename (RuntimeSessionTransientEvent -> RuntimeSessionForwardedEvent) with the corrected comment (forwarding vs persistence being orthogonal) reads accurately against the pump code.

Overlap with #3633 (asked by the orchestrator): the two PRs target the same symptom (#3304 rendering of mid-turn steering) and the same files (session-projector.ts, root-turn-coordinator.ts, execution-host-message.test.ts), but in opposite directions - this PR is additive (forward the durable echo, keep the in-flight synthesis as the rejoin fallback), while #3633 is subtractive (make admission durable, remove the synthesis, remove Desktop's steering IPC). They are complementary rather than competing: whichever lands first, the other must rebase over it. Concretely, if this PR lands first, #3633's rebase would delete the synthesis leg of this PR's three-way dedup (leaving bootstrap + echo, which is exactly #3633's world), and #3633 would inherit this PR's wire schema 5 and epoch. The reverse order leaves this PR's echo as a redundant-but-deduped second render path over #3633's admission durability. Neither order invalidates the other PR's architecture; both orders force a nontrivial rebase of the later one. The bigger practical point: this PR's remaining blocker is mechanical (epoch + rebase), while #3633's NO-GO is substantive (auto-recovery on an unreachable path), so landing this one first does not strand any fix #3633 needs.

Gate: test failed on this exact head (protocol epoch guard). Path-filtered: test is the applicable check; it must be green on the eventual rebased head.

} from './session-transcript.js';

export const SESSION_CONTINUITY_SCHEMA_VERSION = 4 as const;
export const SESSION_CONTINUITY_SCHEMA_VERSION = 5 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] The epoch gate is red on this exact head, and it is the same open item from the 2026-08-22 review.

The CI failure on this head:

Protocol files changed but RUNTIME_HOST_COMPATIBILITY_EPOCH is still <base parent's value>

This PR changes a protocol file (session-continuity.ts, schema 4 -> 5, new event variant) but leaves RUNTIME_HOST_COMPATIBILITY_EPOCH at the base's value. main has since moved to 44, so after rebasing onto current main the epoch must be set past 44 (i.e. 45), with a comment naming this change - not merely rebased onto the old number. The guard exists precisely because same-number sibling bumps merge cleanly and then advertise one epoch for two incompatible protocols (#3313).

Everything else in the wire change looks right (strict exact-keys decode, bounded content, reject-at-open on schema mismatch), so this is the one blocking item.

…ribers

The TUI never rendered a consumed steering message: the host event pump
dropped steering_message (not a transient tool/text event), leaving the
transient in-flight queue observation as the only live render path, which
the coalesced canonical refresh can skip entirely (queued -> consumed).

Forward the durable steering_message on the session-event frame so every
subscriber renders the interjection in place, keep the queue in-flight
synthesis for the attach/rejoin window, and dedup both paths per message.
Bumps the session continuity wire schema to 5.

Fixes apache#3304

Generated-by: Maka
…class

isRuntimeSessionForwardedEvent / RuntimeSessionForwardedEvent describe what
the predicate decides — forward live to subscribers — now that the durable
steering_message belongs to it. Rename the client projectSessionEvent to
match its Host-side counterpart. Addresses review on apache#3316.

Generated-by: Maka
…ady rendered it

subscription.open can bootstrap the durable steering message and install the
subscriber before the Host's forwarded echo arrives; seed the render-dedup
set from steering messages already durable in the transcript so the bootstrapped
render stays the only one. Also rebased onto main and updated the new e2e test
to the post-apache#3277 single-argument connectClient.

Addresses review on apache#3316

Generated-by: Maka
@me2seeks
me2seeks force-pushed the fix/3304-steering-transcript-projection branch from 323fb32 to b874001 Compare August 23, 2026 22:05
@me2seeks

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (c79e9eb4). Epoch is now 45 with the continuity-schema rationale recorded in the epoch history (591d5ff7); the epoch-guard step passes against the new base. The steering-echo suppression tests from the earlier review round are retained and green (session-projector: 11/11).

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review of 591d5ff70c6d2d224c0eb0d2de310781c1cb46c8.

GO. No P0–P2. MERGEABLE / BLOCKED. I am not approving: this head has no terminal-green test run (CI is action_required, typical of a fork PR). Current main test is green, so this is not a main-is-red false alarm.

What this solves

Steering sent mid-turn could be consumed by the model and still vanish from the UI, because the Host only forwarded transient tool/text events and the coalesced canonical refresh can skip in_flight. Forwarding the durable steering_message on subscription.session_event, and deduping it against queue synthesis and transcript bootstrap (durableSteeringMessageIds), is the right cut.

Epoch

Schema 4→5 is a closed-shape change: older peers reject an unknown schemaVersion at admission. That must bump the compatibility epoch; a protocol-compatible-changes/ declaration would be wrong. This head does bump it. After any rebase onto current main, read RUNTIME_HOST_COMPATIBILITY_EPOCH there and take a number greater than that — several open PRs currently share the same next slot.

Prior P1s on this PR (epoch gate, bootstrap dedup, connectClient arity) independently re-judged closed on this head. No inline findings. No merge from me.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 591d5ff7. No P0–P3 on the code. Not approving only because this head has no current-base CI evidence.

The ordering question this change turns on is answered correctly: the steering echo is made durable before it is forwarded (packages/runtime/src/ai-sdk-backend.ts:4532-4547), and the host projector dedupes by messageId across the live, replay and transcript paths (packages/runtime-host/src/adapter/session-projector.ts:361-401), so a reconnect does not double-deliver.

On the red: it is inherited, not attributable to this PR. The failure is packages/storage/src/__tests__/codex-session-adapter.test.ts:267 referencing decodeStoredMessage, which no longer exists. This PR's merge-base is c79e9eb4, which predates #3656 (3bb645e9, test(storage): use the canonical message decoder); current main has zero references to that symbol while the merge-base still has one. The file is outside this PR's diff.

Note that a re-run cannot clear it: re-running replays the same merge ref rather than recomputing against the repaired base. A rebase onto current main is what this needs, after which the checks should speak for the change itself.

中文

591d5ff7 上审,代码面无 P0–P3。不 approve 的唯一原因是本 head 缺少基于当前 base 的 CI 证据。

这个改动最关键的时序问题答对了:steering echo 先落持久化再转发(ai-sdk-backend.ts:4532-4547),且 host projector 在实时、重放、transcript 三条路径上都按 messageId 去重(session-projector.ts:361-401),重连不会重复投递。

关于红:它是继承来的,不归本 PR。失败点是 codex-session-adapter.test.ts:267 引用了已不存在的 decodeStoredMessage。本 PR 的 merge-base 是 c79e9eb4,早于 #36563bb645e9,test(storage): use the canonical message decoder);当前 main 上该符号出现 0 次,而 merge-base 上仍有 1 次,且该文件不在本 PR 的 diff 内。

注意重跑清不掉它:重跑只会重放同一个 merge ref,不会针对修好的 base 重算。需要 rebase 到当前 main,之后 checks 才能真正说明这个改动本身。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(tui): consumed steering message is never projected into the transcript

4 participants